Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

icon view: Fix positioning overflowing icons on the desktop #1678

Merged
merged 1 commit into from
Dec 23, 2022

Conversation

cwendling
Copy link
Member

@cwendling cwendling commented Nov 23, 2022

Properly update the icon data before placing the icon, because positioning might depend on full icon contents on the desktop, whereas updating contents don't care about position.

When an icon position overflows the desktop area, it is clamped to stay in the visible area, but this computation depends on accurate icon and label sizes, which is only available when the icon is fully loaded.

Fix the code to first load the contents and then position instead of the other way around, which was actually trivial.

Note that visible positions were most often correct anyway for two reasons:

  1. Most of the time icons do not overflow, as they are positioned on the final desktop size anyway. It however can easily happen reducing monitor resolution or increasing desktop view zoom. (see also icon view: Refresh icon positions for manual layout on zoom change #1676)
  2. A second layout pass happens most of the time (I'm not yet sure why and when though), but not when an update is triggered before the previous one terminated (e.g. quickly hitting F5 twice).

Properly update the icon data before placing the icon, because
positioning might depend on full icon contents on the desktop, whereas
updating contents don't care about position.

When an icon position overflows the desktop area, it is clamped to stay
in the visible area, but this computation depends on accurate icon and
label sizes, which is only available when the icon is fully loaded.

Fix the code to first load the contents and then position instead of
the other way around, which was actually trivial.

Note that visible positions were most often correct anyway for two
reasons:

1. Most of the time icons do not overflow, as they are positioned on
   the final desktop size anyway.  It however can easily happen
   reducing monitor resolution or increasing desktop view zoom.
2. A second layout pass happens most of the time (I'm not yet sure why
   and when though), but not when an update is triggered before the
   previous one terminated (e.g. quickly hitting F5 twice).
@cwendling cwendling requested a review from a team November 23, 2022 17:35
Copy link
Member

@lukefromdc lukefromdc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't get any visible changes in behavior on changing desktop zoom with this installed (didn't get the issue just before it) but also had no problems. No changes in behavior when dropping an icon onto the desktop, adding a new icon,or removing an icon either.

Not sure why this was ever a separate function unless it was once used elsewhere in the file

@lukefromdc lukefromdc requested a review from a team November 24, 2022 22:23
@lukefromdc
Copy link
Member

Again calling for another review if we can get one

@lukefromdc lukefromdc merged commit 333e272 into master Dec 23, 2022
@lukefromdc lukefromdc deleted the desktop-no-overflow branch December 23, 2022 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants